Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize content of web_monitoring into directories #528

Merged
merged 6 commits into from
Dec 9, 2019

Conversation

Mr0grog
Copy link
Member

@Mr0grog Mr0grog commented Dec 5, 2019

This reorganizes the content of web_monitoring into a hierarchy of modules with several directories based on this discussion: #206 (comment)

web_monitoring/
    tests/
        [same as today]
    diff/
        content_type.py
        differs.py
        diff_errors.py
        html_diff_render.py
        links_diff.py
    diff_server/
        server.py
    cli/                     # Directory to hold CLI command script stuff
        cli.py               # Entrypoint -- parses args and calls other modules
        ia_healthcheck.py    # Currently in scripts folder
        ia_import.py         # Most of what's currently in cli.py
    __init__.py
    _version.py
    utils.py
    db.py
  • diff
  • diff_server (I renamed the module itself to diff_server/server.py)
  • cli
    • Main cli.py file
    • ia_healthcheck script
    • annotations_import script

As part of moving the scripts, I’d like to reconfigure our CLI setup so we only have two scripts: wm and wm-diff-server. The wm command currently has wm import ia and wm import ia-known-pages subcommands, and I’m thinking I’d add wm ia-healthcheck and wm annotations-import. I think there might be room to rethink the command names and structure more, but that’s probably out of scope for this PR (making this script change is already questionably big).

Out of scope, but probably things we might want to do after this PR:

  • Split up the contents of diff_server/server.py.
  • More relative imports everywhere.
  • Reorganize/rename some of the stuff in diff.
  • Break apart the insanity in cli/cli.py a bit.
  • Consider whether any of these new directories should have __init__.py files for easier access to subpackage contents.

@Mr0grog
Copy link
Member Author

Mr0grog commented Dec 5, 2019

@danielballan Would it be a good idea to make the tests directory parallel the structure of these new subpackages?

@Mr0grog
Copy link
Member Author

Mr0grog commented Dec 5, 2019

Actually, nevermind, I don’t think I want to merge the healthcheck and annotation import scripts into the main wm script in this PR.

I still think that’s worth doing, but I realized I probably want to drop docopt and go back to argparse (unless somebody has a strong recommendation for click or cliff or whatever else). Better to keep this PR simple and quick — get things mostly in the right place on disk now so it’s minimally disruptive, then do changes to the code itself in separate PR.

@Mr0grog Mr0grog requested a review from danielballan December 5, 2019 23:35
@Mr0grog Mr0grog marked this pull request as ready for review December 5, 2019 23:35
@Mr0grog Mr0grog merged commit 127a924 into master Dec 9, 2019
@Mr0grog Mr0grog deleted the reorganize-for-rationality branch December 9, 2019 18:41
Mr0grog added a commit that referenced this pull request Dec 10, 2019
In #528, we added subpackages. However, they can't always be imported!

When the list of packages in `setup.py` only included 'web_monitoring', you could import our new subpackages (e.g. `web_monitoring.diff`) from installations that were created via `setup.py develop`, but not `setup.py install` or when installed via pip. This corrects that by listing packages explicitly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant